Skip to content

Conversation

@smklein
Copy link
Collaborator

@smklein smklein commented Oct 14, 2022

This PR re-structures the format of the "package" structure, which also has implications for package-manifest.toml files.

What did we change, and why?

  • Unified internal/external packages: There is information about packages which is shared between internal and external packages, so they've been pulled into the same structure. The "input" for a package is now described by "PackageSource", which can be one of "local" (you're building the package), "prebuilt" (you're downloading the package), "composite" (you're combining packages") or "manual" (you're supplying the package yourself).

  • The boolean "zone" flag has been updated to an enum, called "PackageOutput". This enum describes the different outputs that one should expect from a package.

  • The "Target" evaluation information has been moved to this package from the primary Omicron repository. This makes it easier to act on the 'only_for_target' and 'intermediate_only' fields in a useful way.

This PR re-structures the format of the "package" structure,
which also has implications for package-manifest.toml files.

What did we change, and why?

- Unified internal/external packages: There is information about
packages which is shared between internal and external packages,
so they've been pulled into the same structure. The "input" for a
package is now described by "PackageSource", which can be one
of "local" (you're building the package), "prebuilt" (you're downloading
the package), "composite" (you're combining packages") or "manual"
(you're supplying the package yourself).

- The boolean "zone" flag has been updated to an enum, called
"PackageOutput". This enum describes the different outputs that
one should expect from a package.

- The "Target" evaluation information has been moved to this package
from the primary Omicron repository. This makes it easier to act on the
'only_for_target' and 'intermediate_only' fields in a useful way.
Comment on lines -16 to -28
pub enum ExternalPackageSource {
/// Downloads the package from the following URL:
///
/// <https://buildomat.eng.oxide.computer/public/file/oxidecomputer/REPO/image/COMMIT/PACKAGE>
Prebuilt {
repo: String,
commit: String,
sha256: String,
},
/// Expects that a package will be manually built and placed into the output
/// directory.
Manual,
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything here has been merged into PackageSource below

@smklein smklein requested a review from andrewjstone October 14, 2022 02:59
Copy link
Contributor

@jgallagher jgallagher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Left a handful of comments, only one of which is more than a style/nit suggestion (the question about whether composite packages can contain tarball packages).

Copy link

@andrewjstone andrewjstone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Will be great to have this functionality.


#[tokio::test]
#[tokio::test(flavor = "multi_thread")]
async fn test_composite_package() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test made usage very clear for me. Thank you!

@smklein smklein merged commit 41f6aed into main Oct 17, 2022
@smklein smklein deleted the composite branch October 17, 2022 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants